Skip to content

Conversation

@max-melentyev
Copy link

To let observers know

@jameshartig
Copy link
Contributor

In #1868 we're making Query/Batch reusable. I wonder if we should include those in the observations instead of piecemeal adding a bunch of fields from them. @joao-r-reis thoughts?

@joao-r-reis
Copy link
Contributor

Yeah I agree with that, I can make that change in #1868 but my implementation doesn't make them threadsafe, just reusable so if a user tries to change the query/batch object within an implementation of these observers then bad things might happen...

@max-melentyev
Copy link
Author

Cool! What do you think about adding custom data to a query/batch that is passed to observers? We use a context.WithValue() to pass query names to observer, but this approach can lead to errors because engineers can use the same context for different queries. Would be great to have an approach when data is assigned for individual queries.

@joao-r-reis
Copy link
Contributor

joao-r-reis commented Apr 10, 2025

Cool! What do you think about adding custom data to a query/batch that is passed to observers? We use a context.WithValue() to pass query names to observer, but this approach can lead to errors because engineers can use the same context for different queries. Would be great to have an approach when data is assigned for individual queries.

This seems like a reasonable feature request but I'd recommend creating a new issue/PR for this as it seems unrelated to this one. I'm handling the current one in #1868 by adding the original query/batch objects to the Observed... types.

@joao-r-reis joao-r-reis changed the title Add IsIdempotent to ObservedQuery and ObservedBatch Add Query and Batch to ObservedQuery and ObservedBatch Apr 10, 2025
@max-melentyev max-melentyev changed the title Add Query and Batch to ObservedQuery and ObservedBatch Add IsIdempotent to ObservedQuery and ObservedBatch Apr 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants